Skip to content

EQL: better error message for sequences with only one clause plus UNTIL #132638

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

luigidellaquila
Copy link
Contributor

@luigidellaquila luigidellaquila commented Aug 11, 2025

EQL Sequences need at least two clauses in the query. Parsing time checks didn't take into consideration the fact that UNTIL clause doesn't have to account in this number.

Here we add a specific error for this case.

@elasticsearchmachine elasticsearchmachine added v9.2.0 Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) labels Aug 11, 2025
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-analytical-engine (Team:Analytics)

@elasticsearchmachine
Copy link
Collaborator

Hi @luigidellaquila, I've created a changelog YAML for you.

@luigidellaquila luigidellaquila added v8.19.2 v9.1.2 v9.0.6 auto-backport Automatically create backport pull requests when merged labels Aug 11, 2025
@astefan
Copy link
Contributor

astefan commented Aug 13, 2025

Does this

Also adding a check for usage of missing events in UNTIL.

have a previous report to link to or it's an observation while implementing the fix for until?

@luigidellaquila
Copy link
Contributor Author

have an earlier report to link to or it's an observation while implementing the fix for until?

No earlier reports, I just noticed the problem while fixing the main bug.
Missing events in UNTIL were silently treated as positive events, that is incorrect. I tried to consider a possible meaning of missing events in that context, but I couldn't find a reasonable behavior for them.

@astefan
Copy link
Contributor

astefan commented Aug 13, 2025

Missing events in UNTIL were silently treated as positive events, that is incorrect.

I think this might have some BWC implications. Queries that used to work (incorrectly), will now fail.
Because of this, and if you agree with my statement above, this needs a separate issue, maybe a discussion and then a fix.

@luigidellaquila
Copy link
Contributor Author

this needs a separate issue, maybe a discussion and then a fix.

I tend not to consider it a BWC problem, but rather a bug fix, but treating it as a separate issue and discussing it won't hurt.
I'm reverting that part and opening an issue.

until = queries.remove(queries.size() - 1);
if (until.isMissingEventFilter()) {
throw new ParsingException(source, "UNTIL clause cannot be a negative clause (missing event)");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The correct ParsingException would have been ParsingException(until.source(), "UNTIL clause cannot be a....

Copy link
Contributor

@astefan astefan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with one test suggestion. Thank you!

assertEquals("1:2: A sequence requires a minimum of 2 queries (excluding UNTIL clause), found [1]", s);
plan("sequence [any where true] [any where true] until [any where true]");
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add this query as a test, as well: sequence with maxspan=1h ![process where true] until [process where true]

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @astefan!
I added that test and opened an issue for missing events in UNTIL #132787

@luigidellaquila luigidellaquila enabled auto-merge (squash) August 13, 2025 07:11
@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
8.18
9.0
9.1
8.19

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Analytics/EQL EQL querying auto-backport Automatically create backport pull requests when merged >bug Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v8.18.6 v8.19.3 v9.0.6 v9.1.3 v9.2.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants